SDSTOR-21465: scrubber phase 1#413
Conversation
JacksonYao287
force-pushed
the
scrubber-phase-1
branch
from
0f155d8 to
f3cc39a
Compare
JacksonYao287
requested review from
Besroy,
Hooper9973,
koujl,
xiaoxichen,
yuwmao and
zhiteng
|
Codecov Report❌ Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## stable/v4.x #413 +/- ##
==============================================
Coverage ? 52.93%
==============================================
Files ? 39
Lines ? 6821
Branches ? 931
==============================================
Hits ? 3611
Misses ? 2795
Partials ? 415 ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
JacksonYao287
force-pushed
the
scrubber-phase-1
branch
3 times, most recently
from
b25a7a3 to
83d0375
Compare
JacksonYao287
force-pushed
the
scrubber-phase-1
branch
from
83d0375 to
fdf3a0c
Compare
| "reconcile check: shard {} confirmed absent on peer {}, removing from " | ||
| "existence-tracking set", | ||
| shard_id, peer_id); | ||
| scrub_report->remove_shard_existence_from_peer(shard_id, peer_id); |
There was a problem hiding this comment.
I think there may be an iterator invalidation risk here. missing_shards is returned by const reference, so this loop is walking the original map. If remove_shard_existence_from_peer() erases the current shard_id from that same map, the current range-for element / iterator could be invalidated.
Might be safer to iterate over a snapshot copy, or apply removals after the traversal.
There was a problem hiding this comment.
nice finding, but after checking the code, seems it is safe.
line 1439, use copy constructor to get a new instance missing_shards (this is just like the snapshot you metioned).
const auto missing_shards = scrub_report->get_missing_shard_ids();
so, actually , the behavior is we iterate on a copied new instance of missing_shards( which is never changed), and the removal happens in the source missing_shards( inside scrub_report). since they are totally different missing_shards, so it`s totally safe.
pls correct me if I am wrong
JacksonYao287
force-pushed
the
scrubber-phase-1
branch
from
fdf3a0c to
1cec0d0
Compare
3 participants
No description provided.